Skip to content

feat(coop): permanent soldier ownership transfer + host-authoritative saves#22

Open
NonPolynomialTim wants to merge 3 commits into
xcomcoopdev:mainfrom
NonPolynomialTim:feature/coop-soldier-transfer-pr
Open

feat(coop): permanent soldier ownership transfer + host-authoritative saves#22
NonPolynomialTim wants to merge 3 commits into
xcomcoopdev:mainfrom
NonPolynomialTim:feature/coop-soldier-transfer-pr

Conversation

@NonPolynomialTim

@NonPolynomialTim NonPolynomialTim commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a co-op feature to permanently transfer soldier ownership between the
two players, backed by a rework of how co-op games are saved so those
transfers survive save/load and rollbacks.
$${\color{red}NOTE:\ This\ is\ very\ likely\ a\ breaking\ change\ for\ saves.}$$

This is squashed from the feature development branch into a single commit.

Soldier ownership transfer

  • New "Give unit" dialog (TransferSoldierMenu) reachable from the soldier
    list, the craft-soldier list and the soldier-info screen, plus in battle.
  • Guest-soldier model: a soldier's object lives in its owner's save,
    tagged with the station base's coop id. Transferring serialises the soldier,
    removes it from the giver's save and recreates it in the receiver's save,
    keeping the same station base — so it stays "in" the base it was in and shows
    up when the new owner views that base.
  • In battle only the control flags flip immediately; the physical move is
    queued and applied once the mission ends.
  • Duplicate-delivery guard keyed on a sender-unique xfer_id. Roster-based
    checks are unreliable: two fresh saves both number soldiers from 1 and can
    roll identical names, which made a genuine transfer look like a duplicate.
  • On-screen transfer notice (TransferNoticeState), geoscape-legible, with
    the correct player names and instant visibility during peer base visits.

Save-file rework (single authority)

  • The host's .sav embeds the latest client-world blob, so loading a host
    save atomically restores both players' rosters; the client re-fetches its
    world from the host on reconnect.
  • Because both worlds travel together, loading rolls both sides' transfers
    back in lockstep
    , making trades rollback-safe.
  • Session transfer state (pending queues, dedup ids, away-ids) is reset on load
    so stale in-memory state never outlives the save that is now the authority.

Notes

  • Build verified: Release | x64 compiles cleanly against this base
    (OpenXcom.exe links, 0 errors).

… saves

Adds a co-op feature to permanently hand a soldier to the other player,
backed by a rework of how co-op games are persisted.

Transfer:
- New "Give unit" dialog (TransferSoldierMenu) reachable from the soldier
  list, craft-soldier list and soldier-info screens, plus in battle.
- Guest-soldier model: a soldier's object lives in its owner's save, tagged
  with the station base's coop id; transferring serializes it, removes it
  from the giver and recreates it in the receiver while keeping the same
  station base, so it stays where it was and shows up for the new owner.
- In battle only the control flags flip immediately; the physical move is
  queued and applied once the mission ends.
- Duplicate-delivery guard keyed on a sender-unique xfer_id (roster-based
  checks are unreliable: two fresh saves both number soldiers from 1 and can
  roll identical names).
- On-screen transfer notice (TransferNoticeState), geoscape-legible, with
  correct player names and instant visibility during peer base visits.

Save-file rework (single authority):
- The host's .sav embeds the latest client-world blob, so loading a host
  save atomically restores BOTH rosters; the client re-fetches its world on
  reconnect. Loading rolls both sides' transfers back together, making trades
  rollback-safe. Session transfer state is reset on load.

Squashed from the feature branch; test harness intentionally excluded.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@NonPolynomialTim NonPolynomialTim marked this pull request as draft July 7, 2026 09:08
@NonPolynomialTim

NonPolynomialTim commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

[EDIT: FIXED] Bug: Battlescape transfers do not persist, leading to the soldier being lost to both players.

@NonPolynomialTim

NonPolynomialTim commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

[EDIT: FIXED] Bug: Transferred soldier's equipped craft not persisted after first mission post-transfer

…tle transfer

Fixes two bugs in the coop soldier-transfer flow.

Bug 1 (guest unassigned from host craft): the client's guest -> host-craft
assignment was written only into the "basehost" blob (its copy of the host
world). The mission-end own-world reload (GeoscapeState) reads the client's
own-world blob, which never saw the assignment, so CoopCraft reverted to its
stale value and the soldier came back unassigned. Add
connectionTCP::syncOwnWorldGuestCraft() to durably mirror each guest's
CoopCraft/CoopCraftType into the own-world blob, wired from
SoldiersState::btnOkClick after the basehost save.

Bug 2 (soldier transferred mid-battle lost entirely): at mission end the
client's live save is still the host's throwaway battle world, so the deferred
physical hand-off was applied there (coopBase cleared, soldier deleted by the
post-battle cleanup) and pushProgressToHostSilently() then uploaded that host
world as the client's own-world blob, destroying the roster. Defer soldier_yaml
transfers that arrive during the mission-end / active-battle window and replay
them once the own world is restored; guard pushProgressToHostSilently() and the
incoming-transfer replay against the swapped-world window (mission-end,
savedBattle, or a pending LoadGameState).

Also, in processPendingSoldierTransfers(): auto-keep an in-battle-transferred
soldier on the craft it was deployed on (unless wounded) via the guest CoopCraft
mechanism, and reset a fallen transfer to a plain own-soldier so the giver's
memorial records it correctly.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@NonPolynomialTim NonPolynomialTim marked this pull request as ready for review July 8, 2026 01:21
sackSoldier's base-palette colors are illegible under the battle palette. In
battle, match the coop lobby window: setInterface("geoscape", true, battle) +
saveMenus element colors + applyBattlescapeTheme("saveMenus") for the uniform
battlescape theme color, high contrast and TAC00 background. Out-of-battle
behavior is unchanged (sackSoldier over basescape, palette-adopt for the notice).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@TheYellowArchitect

TheYellowArchitect commented Jul 8, 2026

Copy link
Copy Markdown

Insane feature and very glad to see it. These 2 especially 👌

On-screen transfer notice (TransferNoticeState), geoscape-legible, with
the correct player names and instant visibility during peer base visits.

New "Give unit" dialog (TransferSoldierMenu) reachable from the soldier
list, the craft-soldier list and the soldier-info screen, plus in battle.

so as long this implementation is clean, it's so worth it 🚀


For no misunderstandings when talking about this feature, just like other games, it is good to seperate the word controller and owner, because I think this design completely tries to do away with controller and simply has owner change, specifically

Guest-soldier model: a soldier's object lives in its owner's save,
tagged with the station base's coop id. Transferring serialises the soldier,
removes it from the giver's save and recreates it in the receiver's save,
keeping the same station base — so it stays "in" the base it was in and shows
up when the new owner views that base.

Why would a soldier in the base of P1, be removed from the savefile of P1 and exist in the save of P2?
Doesn't this prevent a multiplayer campaign continuing in singleplayer? (e.g. the other player loses interest in the campaign or will be busy for a month)

Also, having everything in the base of P1, exist only in the savefile of P1 is good for simplicity/clarity and by extension, debugging.

Aside of debugging crashes and bugs being made harder, one more thing it makes harder is the planned implementation of 4 players. If their save files are intertwined together, debugging them will be very hard.

Duplicate-delivery guard keyed on a sender-unique xfer_id

Could you update this part, because with the latest commits, there is no xfer_id variable in the code changes

@NonPolynomialTim

Copy link
Copy Markdown
Contributor Author

@TheYellowArchitect

For no misunderstandings when talking about this feature, just like other games, it is good to seperate the word controller and owner, because I think this design completely tries to do away with controller and simply has owner change

You are correct, this does away with the controller concept, and that is intentional. Currently, you can loan soldiers, but you cannot gift them. With this change you can do both with the same mechanism since soldiers are re-transferrable. If you want to give temporary control of a unit you can give the unit to your teammate, and your teammate can give the unit back after the mission. I think supporting both paths/mechanisms is overkill, especially for a community-driven open source project.

Why would a soldier in the base of P1, be removed from the savefile of P1 and exist in the save of P2?
Doesn't this prevent a multiplayer campaign continuing in singleplayer? (e.g. the other player loses interest in the campaign or will be busy for a month)

Yes, but very few games support this flow because it is incredibly error-prone to have 2 separate sources of truth that can operate independently but then must be merged. My assumption with this PR is that players would not continue a campaign in another player's absence, because that requires maintaining custom world-merge code which gets messy. The two saves are already fully coupled before this PR: transferring soldiers (the vanilla X-COM mechanic) across base-ownership lines requires that both saves know about both bases, both online and offline. When those two separate copies of two separate bases come together it is going to be messy and require deep, brittle merge logic, which only gets messier and more brittle when scaled up to 4 independent saves trying to agree on the current game state.

Also, having everything in the base of P1, exist only in the savefile of P1 is good for simplicity/clarity and by extension, debugging.

Same argument as above (separated saves are brittle), but with the added point that the host embeds the client save, so the host can continue the game solo without any issue with the full roster of soldiers, only P2 cannot continue solo.

Aside of debugging crashes and bugs being made harder, one more thing it makes harder is the planned implementation of 4 players. If their save files are intertwined together, debugging them will be very hard.

Again, same as above, separating the saves makes this much harder. We can either:

  1. Intertwine at game-event-time knowing the full state of the game from a single source of truth to know what the consequences of the game event should be and how to track them in the single source of truth
  2. We can try to intertwine at every place in the code that game events affect, and try to merge the states of all saves to decide on-the-fly what the effective truth is based on the infinite combination of game states presented by both (or all four) players

Duplicate-delivery guard keyed on a sender-unique xfer_id

Could you update this part, because with the latest commits, there is no xfer_id variable in the code changes

xfer_id is the name of the json key. The C++variable is xferId. The description is accurate

I would appreciate @xcomcoopdev's thoughts on the save architecture here though regardless, because it is an architectural change (but in the direction that I believe they were headed already based on the changes to how saves work from 1.7.0 => 1.8.4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants